rework gathering of serialization information. (#479)
authortsteven4 <13596209+tsteven4@users.noreply.github.com>
Tue, 28 Jan 2020 02:08:51 +0000 (19:08 -0700)
committerGitHub <noreply@github.com>
Tue, 28 Jan 2020 02:08:51 +0000 (19:08 -0700)
The existing method broke encapsulation to generate the combined
list of regular and style based formats.  It required access to
xcsv_vecs, which is only present in LegacyFormats.  Until recently
it also required access to xcsv_file.

The existing method also leaked memory, although the use case was
such that the program always exited shortly thereafter.

All these issues are resolved during unification by gathering all the
necessary information and passing that on to serialization.  This
information is gathered from different places for regular and style
based formats.

testo.d/serialization.test
vecs.cc
vecs.h

index 9423a8d740159a58e3cbd8bf312e2715bc8fcf9b..491bb94b092d1dff2484a8413f244cea115ce0fe 100755 (executable)
@@ -3,38 +3,34 @@
 # These outputs will change with any new filter or changed filter option.
 # Filter options must be hand coded in the GUI.
 #
-# we have memory leaks in sort_and_unify.
-if [ ${RUNNINGVALGRIND} -ne  0 ]; then
-  # These are primarily meant to serialize the interface specification to
-  # the GUI and the document.
-  # We do a compare_nole as specific whitespace is part of deserialization.
-  gpsbabel -^3 > ${TMPDIR}/format3.txt
-  compare_nole ${REFERENCE}/format3.txt ${TMPDIR}/format3.txt
-  gpsbabel -^2 > ${TMPDIR}/format2.txt
-  compare_nole ${REFERENCE}/format2.txt ${TMPDIR}/format2.txt
-  gpsbabel -^1 > ${TMPDIR}/format1.txt
-  compare_nole ${REFERENCE}/format1.txt ${TMPDIR}/format1.txt
-  gpsbabel -^0 > ${TMPDIR}/format0.txt
-  compare_nole ${REFERENCE}/format0.txt ${TMPDIR}/format0.txt
-  gpsbabel -%1 > ${TMPDIR}/filter1.txt
-  compare_nole ${REFERENCE}/filter1.txt ${TMPDIR}/filter1.txt
-  gpsbabel -%0 > ${TMPDIR}/filter0.txt
-  compare_nole ${REFERENCE}/filter0.txt ${TMPDIR}/filter0.txt
-  
-  # These are primarily meant for a user
-  gpsbabel -h > ${TMPDIR}/help.txt
-  # ahh shucks, the executable changes based on OS, and the path can change as well.
-  sed 's/.*\[options\]/    .\/gpsbabel [options]/' ${TMPDIR}/help.txt > ${TMPDIR}/help.stripped.txt
-  compare ${REFERENCE}/help.txt ${TMPDIR}/help.stripped.txt
-  gpsbabel -h gpx > ${TMPDIR}/formatusage.txt
-  compare ${REFERENCE}/formatusage.txt ${TMPDIR}/formatusage.txt
-  gpsbabel -h radius > ${TMPDIR}/filterusage.txt
-  compare ${REFERENCE}/filterusage.txt ${TMPDIR}/filterusage.txt
-  gpsbabel > ${TMPDIR}/usage.txt << EOJ
-  
-EOJ
-  # ahh shucks, the executable changes based on OS, and the path can change as well.
-  sed 's/.*\[options\]/    .\/gpsbabel [options]/' ${TMPDIR}/usage.txt > ${TMPDIR}/usage.stripped.txt
-  compare ${REFERENCE}/usage.txt ${TMPDIR}/usage.stripped.txt
-fi
+# These are primarily meant to serialize the interface specification to
+# the GUI and the document.
+# We do a compare_nole as specific whitespace is part of deserialization.
+gpsbabel -^3 > ${TMPDIR}/format3.txt
+compare_nole ${REFERENCE}/format3.txt ${TMPDIR}/format3.txt
+gpsbabel -^2 > ${TMPDIR}/format2.txt
+compare_nole ${REFERENCE}/format2.txt ${TMPDIR}/format2.txt
+gpsbabel -^1 > ${TMPDIR}/format1.txt
+compare_nole ${REFERENCE}/format1.txt ${TMPDIR}/format1.txt
+gpsbabel -^0 > ${TMPDIR}/format0.txt
+compare_nole ${REFERENCE}/format0.txt ${TMPDIR}/format0.txt
+gpsbabel -%1 > ${TMPDIR}/filter1.txt
+compare_nole ${REFERENCE}/filter1.txt ${TMPDIR}/filter1.txt
+gpsbabel -%0 > ${TMPDIR}/filter0.txt
+compare_nole ${REFERENCE}/filter0.txt ${TMPDIR}/filter0.txt
+
+# These are primarily meant for a user
+gpsbabel -h > ${TMPDIR}/help.txt
+# ahh shucks, the executable changes based on OS, and the path can change as well.
+sed 's/.*\[options\]/    .\/gpsbabel [options]/' ${TMPDIR}/help.txt > ${TMPDIR}/help.stripped.txt
+compare ${REFERENCE}/help.txt ${TMPDIR}/help.stripped.txt
+gpsbabel -h gpx > ${TMPDIR}/formatusage.txt
+compare ${REFERENCE}/formatusage.txt ${TMPDIR}/formatusage.txt
+gpsbabel -h radius > ${TMPDIR}/filterusage.txt
+compare ${REFERENCE}/filterusage.txt ${TMPDIR}/filterusage.txt
+gpsbabel > ${TMPDIR}/usage.txt << EOJ
 
+EOJ
+# ahh shucks, the executable changes based on OS, and the path can change as well.
+sed 's/.*\[options\]/    .\/gpsbabel [options]/' ${TMPDIR}/usage.txt > ${TMPDIR}/usage.stripped.txt
+compare ${REFERENCE}/usage.txt ${TMPDIR}/usage.stripped.txt
diff --git a/vecs.cc b/vecs.cc
index 58c40b5ac75e633f023af7cfce30ef5d4cae31af..500ebb80d6d7d183042f0c712645d90fa6d0e19b 100644 (file)
--- a/vecs.cc
+++ b/vecs.cc
@@ -360,76 +360,88 @@ QString Vecs::get_option(const QStringList& options, const char* argname)
 }
 
 /*
- * Smoosh the vecs list and style lists together and sort them
- * alphabetically.  Returns an allocated copy of a style_vecs_array
- * that's populated and sorted.
+ * Gather information relevant to serialization from the
+ * vecs and style lists.  Sort and return the information.
  */
-QVector<Vecs::vecs_t> Vecs::sort_and_unify_vecs() const
+QVector<Vecs::vecinfo_t> Vecs::sort_and_unify_vecs() const
 {
-  QVector<vecs_t> svp;
+  QVector<vecinfo_t> svp;
   svp.reserve(vec_list.size() + style_list.size());
 
-  /* Normal vecs are easy; populate the first part of the array. */
+  /* Gather relevant information for normal formats. */
   for (const auto& vec : vec_list) {
-    vecs_t uvec = vec;
-    if (uvec.parent == nullptr) {
-      uvec.parent = uvec.name;
+    vecinfo_t info;
+    info.name = vec.name;
+    info.desc = vec.desc;
+    info.extensions = vec.extensions;
+    if (vec.parent.isEmpty()) {
+      info.parent = vec.name;
+    } else {
+      info.parent = vec.parent;
     }
-    svp.append(uvec);
+    info.type = vec.vec->get_type();
+    info.cap = vec.vec->get_cap();
+    const QVector<arglist_t>* args = vec.vec->get_args();
+    if (args != nullptr) {
+      for (const auto& arg : *args) {
+        info.arginfo.append(arginfo_t(arg));
+      }
+    }
+    svp.append(info);
   }
 
 #if CSVFMTS_ENABLED
   /* The style formats are based on the xcsv format,
    * Make sure we know which entry in the vector list that is.
    */
-  assert(vec_list.at(0).name == "xcsv");
+  assert(vec_list.at(0).name.compare("xcsv", Qt::CaseInsensitive) == 0);
   /* The style formats use a modified xcsv argument list that doesn't include
    * the option to set the style file.  Make sure we know which entry in
    * the argument list that is.
    */
   assert(case_ignore_strcmp(vec_list.at(0).vec->get_args()->at(0).helpstring,
                             "Full path to XCSV style file") == 0);
-  /* Prepare a modified argument list for the style formats. */
-  auto xcsv_args = new QVector<arglist_t>(*vec_list.at(0).vec->get_args()); /* LEAK */
-  xcsv_args->removeFirst();
 
-  /* Walk the style list, parse the entries, dummy up a "normal" vec */
+  /* Gather the relevant info for the style based formats. */
   for (const auto& svec : style_list) {
     XcsvStyle style = xcsv_read_internal_style(svec.style_buf);
-    vecs_t uvec;
-    uvec.name = svec.name;
-    uvec.extensions = style.extension;
-    /* TODO: This needs to be reworked when xcsv isn't a LegacyFormat and
-     * xcsv_vecs disappear.
-     */
-    auto ffvec = ff_vecs_t(xcsv_vecs); /* Inherits xcsv opts */
-    /* Reset file type to inherit ff_type from xcsv. */
-    ffvec.type = style.type;
-    /* Skip over the first help entry for all but the
-     * actual 'xcsv' format - so we don't expose the
-     * 'Full path to XCSV style file' argument to any
-     * GUIs for an internal format.
-     */
-    ffvec.args = xcsv_args;
-    ffvec.cap.fill(ff_cap_none);
+    vecinfo_t info;
+    info.name = svec.name;
+    info.desc = style.description;
+    info.extensions = style.extension;
+    info.parent = "xcsv";
+    info.type = style.type;
+    info.cap.fill(ff_cap_none, 3);
     switch (style.datatype) {
     case unknown_gpsdata:
     case wptdata:
-      ffvec.cap[ff_cap_rw_wpt] = (ff_cap)(ff_cap_read | ff_cap_write);
+      info.cap[ff_cap_rw_wpt] = (ff_cap)(ff_cap_read | ff_cap_write);
       break;
     case trkdata:
-      ffvec.cap[ff_cap_rw_trk] = (ff_cap)(ff_cap_read | ff_cap_write);
+      info.cap[ff_cap_rw_trk] = (ff_cap)(ff_cap_read | ff_cap_write);
       break;
     case rtedata:
-      ffvec.cap[ff_cap_rw_rte] = (ff_cap)(ff_cap_read | ff_cap_write);
+      info.cap[ff_cap_rw_rte] = (ff_cap)(ff_cap_read | ff_cap_write);
       break;
     default:
       ;
     }
-    uvec.vec = new LegacyFormat(ffvec); /* LEAK */
-    uvec.desc = style.description;
-    uvec.parent = "xcsv";
-    svp.append(uvec);
+    /* Skip over the first help entry of the xcsv args.
+     * We don't want to expose the
+     * 'Full path to XCSV style file' argument to any
+     * GUIs for an internal format.
+     */
+    bool first = true;
+    const QVector<arglist_t>* args = vec_list.at(0).vec->get_args();
+    if (args != nullptr) {
+      for (const auto& arg : *args) {
+        if (!first) {
+          info.arginfo.append(arginfo_t(arg));
+        }
+        first = false;
+      }
+    }
+    svp.append(info);
   }
 #endif // CSVFMTS_ENABLED
 
@@ -437,7 +449,7 @@ QVector<Vecs::vecs_t> Vecs::sort_and_unify_vecs() const
    *  Display the available formats in a format that's easy for humans to
    *  parse for help on available command line options.
    */
-  auto alpha = [](const vecs_t& a, const vecs_t& b)->bool {
+  auto alpha = [](const vecinfo_t& a, const vecinfo_t& b)->bool {
     return QString::compare(a.desc, b.desc, Qt::CaseInsensitive) < 0;
   };
 
@@ -453,20 +465,19 @@ void Vecs::disp_vecs() const
 {
   const auto svp = sort_and_unify_vecs();
   for (const auto& vec : svp) {
-    if (vec.vec->get_type() == ff_type_internal)  {
+    if (vec.type == ff_type_internal)  {
       continue;
     }
     printf(VEC_FMT, qPrintable(vec.name), qPrintable(vec.desc));
-    const QVector<arglist_t>* args = vec.vec->get_args();
-    if (args) {
-      for (const auto& arg : *args) {
-        if (!(arg.argtype & ARGTYPE_HIDDEN))
-          printf("       %-18.18s    %s%-.50s %s\n",
-                 arg.argstring,
-                 (arg.argtype & ARGTYPE_TYPEMASK) ==
-                 ARGTYPE_BOOL ? "(0/1) " : "",
-                 arg.helpstring,
-                 (arg.argtype & ARGTYPE_REQUIRED) ? "(required)" : "");
+    const QVector<arginfo_t> args = vec.arginfo;
+    for (const auto& arg : args) {
+      if (!(arg.argtype & ARGTYPE_HIDDEN)) {
+        printf("         %-18.18s    %s%-.50s %s\n",
+               qPrintable(arg.argstring),
+               (arg.argtype & ARGTYPE_TYPEMASK) ==
+               ARGTYPE_BOOL ? "(0/1) " : "",
+               qPrintable(arg.helpstring),
+               (arg.argtype & ARGTYPE_REQUIRED) ? "(required)" : "");
       }
     }
   }
@@ -481,16 +492,15 @@ void Vecs::disp_vec(const QString& vecname) const
     }
 
     printf(VEC_FMT, qPrintable(vec.name), qPrintable(vec.desc));
-    const QVector<arglist_t>* args = vec.vec->get_args();
-    if (args) {
-      for (const auto& arg : *args) {
-        if (!(arg.argtype & ARGTYPE_HIDDEN))
-          printf("       %-18.18s    %s%-.50s %s\n",
-                 arg.argstring,
-                 (arg.argtype & ARGTYPE_TYPEMASK) ==
-                 ARGTYPE_BOOL ? "(0/1) " : "",
-                 arg.helpstring,
-                 (arg.argtype & ARGTYPE_REQUIRED) ? "(required)" : "");
+    const QVector<arginfo_t> args = vec.arginfo;
+    for (const auto& arg : args) {
+      if (!(arg.argtype & ARGTYPE_HIDDEN)) {
+        printf("         %-18.18s    %s%-.50s %s\n",
+               qPrintable(arg.argstring),
+               (arg.argtype & ARGTYPE_TYPEMASK) ==
+               ARGTYPE_BOOL ? "(0/1) " : "",
+               qPrintable(arg.helpstring),
+               (arg.argtype & ARGTYPE_REQUIRED) ? "(required)" : "");
       }
     }
   }
@@ -521,9 +531,9 @@ void Vecs::disp_v1(ff_type t)
   printf("%s\t", tstring);
 }
 
-void Vecs::disp_v2(const Format* v)
+void Vecs::disp_v2(const vecinfo_t& v)
 {
-  for (auto& i : v->get_cap()) {
+  for (auto& i : v.cap) {
     putchar((i & ff_cap_read) ? 'r' : '-');
     putchar((i & ff_cap_write) ? 'w' : '-');
   }
@@ -548,35 +558,33 @@ const char* Vecs::name_option(uint32_t type)
   return at[0];
 }
 
-void Vecs::disp_help_url(const vecs_t& vec, const arglist_t* arg)
+void Vecs::disp_help_url(const vecinfo_t& vec, const QString& argstring)
 {
   printf("\t" WEB_DOC_DIR "/fmt_%s.html", CSTR(vec.name));
-  if (arg) {
-    printf("#fmt_%s_o_%s", CSTR(vec.name), arg->argstring);
+  if (!argstring.isEmpty()) {
+    printf("#fmt_%s_o_%s", CSTR(vec.name), CSTR(argstring));
   }
   printf("\n");
 }
 
 
-void Vecs::disp_v3(const Vecs::vecs_t& vec)
+void Vecs::disp_v3(const vecinfo_t& vec)
 {
   disp_help_url(vec, nullptr);
-  const QVector<arglist_t>* args = vec.vec->get_args();
-  if (args) {
-    for (const auto& arg : *args) {
-      if (!(arg.argtype & ARGTYPE_HIDDEN)) {
-        printf("option\t%s\t%s\t%s\t%s\t%s\t%s\t%s",
-               CSTR(vec.name),
-               arg.argstring,
-               arg.helpstring,
-               name_option(arg.argtype),
-               arg.defaultvalue ? arg.defaultvalue : "",
-               arg.minvalue ? arg.minvalue : "",
-               arg.maxvalue ? arg.maxvalue : "");
-      }
-      disp_help_url(vec, &arg);
-      printf("\n");
+  const QVector<arginfo_t> args = vec.arginfo;
+  for (const auto& arg : args) {
+    if (!(arg.argtype & ARGTYPE_HIDDEN)) {
+      printf("option\t%s\t%s\t%s\t%s\t%s\t%s\t%s",
+             CSTR(vec.name),
+             CSTR(arg.argstring),
+             CSTR(arg.helpstring),
+             name_option(arg.argtype),
+             arg.defaultvalue.isEmpty() ? "" : CSTR(arg.defaultvalue),
+             arg.minvalue.isEmpty() ? "" : CSTR(arg.minvalue),
+             arg.maxvalue.isEmpty() ? "" : CSTR(arg.maxvalue));
     }
+    disp_help_url(vec, arg.argstring);
+    printf("\n");
   }
 }
 
@@ -598,14 +606,14 @@ void Vecs::disp_formats(int version) const
        * Version 0 skips internal types.
        */
       if (version > 0) {
-        disp_v1(vec.vec->get_type());
+        disp_v1(vec.type);
       } else {
-        if (vec.vec->get_type() == ff_type_internal) {
+        if (vec.type == ff_type_internal) {
           continue;
         }
       }
       if (version >= 2) {
-        disp_v2(vec.vec);
+        disp_v2(vec);
       }
       printf("%s\t%s\t%s%s%s\n", CSTR(vec.name),
              !vec.extensions.isEmpty() ? CSTR(vec.extensions) : "",
diff --git a/vecs.h b/vecs.h
index 5849cfcb484dd9f04cd7c8039cf9ec5760718a13..2d2870a6a2a305b9cd0d07dd3bf77ac8728f9670 100644 (file)
--- a/vecs.h
+++ b/vecs.h
@@ -206,6 +206,35 @@ private:
     QString parent;
   };
 
+  struct arginfo_t {
+    arginfo_t() = default;
+    explicit arginfo_t(const arglist_t& arg) :
+      argstring(arg.argstring),
+      helpstring(arg.helpstring),
+      defaultvalue(arg.defaultvalue),
+      argtype(arg.argtype),
+      minvalue(arg.minvalue),
+      maxvalue(arg.maxvalue)
+    {}
+
+    QString argstring;
+    QString helpstring;
+    QString defaultvalue;
+    uint32_t argtype{ARGTYPE_UNKNOWN};
+    QString minvalue;
+    QString maxvalue;
+  };
+
+  struct vecinfo_t {
+    QString name;
+    QString desc;
+    QString extensions;
+    QString parent;
+    ff_type type{ff_type_file};
+    QVector<ff_cap> cap;
+    QVector<arginfo_t> arginfo;
+  };
+
 public:
   void init_vecs();
   void exit_vecs();
@@ -223,11 +252,11 @@ public:
 
 private:
   static int is_integer(const char* c);
-  QVector<vecs_t> sort_and_unify_vecs() const;
+  QVector<vecinfo_t> sort_and_unify_vecs() const;
   static void disp_v1(ff_type t);
-  static void disp_v2(const Format* v);
-  static void disp_help_url(const vecs_t& vec, const arglist_t* arg);
-  static void disp_v3(const vecs_t& vec);
+  static void disp_v2(const vecinfo_t& v);
+  static void disp_help_url(const vecinfo_t& vec, const QString& argstring);
+  static void disp_v3(const vecinfo_t& vec);
   static bool validate_vec(const vecs_t& vec);
 
 private: